Add favicon to Volcano website#508
Conversation
volcano-sh-bot
added
kind/feature
|
Welcome @kubeboiii! It looks like this is your first PR to volcano-sh/website 🎉 |
volcano-sh-bot
added
the
size/M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request configures favicons and metadata for the website by adding a favicon path and various head tags (apple-touch-icon, manifest, mask-icon) to docusaurus.config.js, and updating the name in site.webmanifest. The reviewer noted that while the PR description mentions copying favicon.ico to the site root, the physical static/favicon.ico file is missing from the changes and needs to be added.
| url: "https://volcano.sh", | ||
| baseUrl: "/", | ||
|
|
||
| favicon: "img/favicons/favicon.ico", |
There was a problem hiding this comment.
The PR description states that this change "copies favicon.ico to the site root for clients that request /favicon.ico". However, there is no static/favicon.ico file included in this pull request, nor are there any build scripts or plugins configured to perform this copy.
To make the favicon available at the root /favicon.ico (which many browsers and crawlers query by default), please physically copy the favicon.ico file to static/favicon.ico and commit it as part of this PR.
There was a problem hiding this comment.
static/favicon.ico is already in this PR (see Files changed, commit 7f89bb7). It is a direct copy of static/img/favicons/favicon.ico and is served at /favicon.ico via Docusaurus static assets. No build step needed.
The site already had favicon files under static/img/favicons but Docusaurus was not configured to emit them in the document head. Wire favicon, apple-touch, manifest, and Safari mask icon links. Signed-off-by: Himanshu <himanshuwankhadeh@gmail.com>
kubeboiii
force-pushed
the
fix/507-add-favicon
branch
from
4857921 to
7f89bb7
Compare
volcano-sh-bot
removed
the
do-not-merge/invalid-commit-message
|
/lgtm from my side .. @JesseStutler |
|
What's the effect? I didn't see it in netlify preview |
Sir, click on deploy/netlify to see netlify deployment .. |
OK I have seen it now in the bookmark: |
@JesseStutler i'm able to see favicon icon in netlify deploy.. |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JesseStutler The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
volcano-sh-bot
added
the
approved
4 participants
/kind feature
The Volcano website did not show a favicon in the browser tab. Favicon
assets already existed under
static/img/favicons/but were notreferenced in
docusaurus.config.js. This change wires the favicon inDocusaurus config, adds head tags for apple-touch-icon and web manifest,
copies
favicon.icoto the site root for clients that request/favicon.ico, and sets the manifest display name to Volcano.Fixes #507
Test plan
npm run buildcompletes successfully<link rel="icon" href="/img/favicons/favicon.ico">build/favicon.icois present after build